Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(store-sync): fix overflowing column types, bump postgres sync version #2270

Merged
merged 9 commits into from
Feb 20, 2024

Conversation

yonadaa
Copy link
Contributor

@yonadaa yonadaa commented Feb 16, 2024

@yonadaa yonadaa requested review from alvrs and holic as code owners February 16, 2024 12:01
Copy link

changeset-bot bot commented Feb 16, 2024

🦋 Changeset detected

Latest commit: 0ce8e4d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
@latticexyz/store-sync Patch
@latticexyz/dev-tools Patch
@latticexyz/store-indexer Patch
@latticexyz/abi-ts Patch
@latticexyz/block-logs-stream Patch
@latticexyz/cli Patch
@latticexyz/common Patch
@latticexyz/config Patch
create-mud Patch
@latticexyz/ecs-browser Patch
@latticexyz/faucet Patch
@latticexyz/gas-report Patch
@latticexyz/network Patch
@latticexyz/noise Patch
@latticexyz/phaserx Patch
@latticexyz/protocol-parser Patch
@latticexyz/react Patch
@latticexyz/recs Patch
@latticexyz/schema-type Patch
@latticexyz/services Patch
@latticexyz/solecs Patch
solhint-config-mud Patch
solhint-plugin-mud Patch
@latticexyz/std-client Patch
@latticexyz/std-contracts Patch
@latticexyz/store-cache Patch
@latticexyz/store Patch
@latticexyz/utils Patch
@latticexyz/world-modules Patch
@latticexyz/world Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

case "int24":
case "int32":
// integer = 4 bytes (https://www.postgresql.org/docs/current/datatype-numeric.html#DATATYPE-INT)
return asNumber(name, "integer");

case "uint32":
case "uint40":
case "uint48":
case "int40":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't comment below but does int48 need to be bumped to the next size up too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't comment below

What does that mean sorry? Anyway I don't think so, int48 is already a postgres bigint here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We chatted IRL and we mixed up the JavaScript and Postgres column types.

but maybe uint64 and int64 need to move down because they may not fit within the bigint column type

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@holic it definitely doesn't fit - uint64 is 8 bytes unsigned, postgres bigint is 8 bytes signed:

https://www.postgresql.org/docs/current/datatype-numeric.html

@yonadaa yonadaa changed the title chore(store-sync): bump postgres indexer version chore(store-sync): bump postgres indexer version, fix uint32 size Feb 16, 2024
holic
holic previously approved these changes Feb 20, 2024
@holic holic changed the title chore(store-sync): bump postgres indexer version, fix uint32 size fix(store-sync): bump postgres indexer version, fix uint32 size Feb 20, 2024
@holic holic changed the title fix(store-sync): bump postgres indexer version, fix uint32 size fix(store-sync): fix overflowing column types, bump postgres sync version Feb 20, 2024
@holic holic merged commit 6c615b6 into main Feb 20, 2024
10 of 11 checks passed
@holic holic deleted the yonadaaa/indexer-datatypes-bump branch February 20, 2024 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants